-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-15228 ("RoundFunction" which extends StandardSQLFunction to return BigDecimal) #4987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for your pull request! This pull request appears to follow the contribution rules. › This message was automatically generated. |
| return firstArgumentType == StandardBasicTypes.BIG_DECIMAL ? firstArgumentType : StandardBasicTypes.DOUBLE; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a general rule, PR without testing case has slim chance to be approved (with the exception of doc PR), let alone merged.
Please consider creating a compelling testing case(s) to prove your code changes. It would help both reviewer and reviewee at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @NathanQingyangXu I have added unit tests for same.
|
Not sure I think we should be making piecemeal changes like this to Hibernate 5. This is all already fixed Hibernate 6 where all this is much more well-defined, and if you're stuck on 5 you can pretty do this customization in your own project. |
|
In unsupported (on H2 side) H2 1.4.200 it returns value of the same data type for In unsupported (on H2 side) H2 1.4.199 and older versions it returns a Old implementation looks like correct for H2 1.4.199 and older versions and incorrect for newer versions. Implementation proposed here looks like incorrect for all versions of H2, but it may work better anyway. |
|
But the correct (portable) behavior is now defined by JPA, and is not something that is supposed to vary between databases. Again, I don't think we need to be messing with the existing behavior in H5. This was all already carefully reviewed for H6. |
|
@gavinking but it behaves differently with postgres. We are using postgres database production environment, it returns the data type same as argument, but H2 doesn't for same version of hibernate. So currently, there are different behaviours for H2 and postgres (I assume others database as well). |
|
Again: if you're talking about Hibernate 5 here, then yes, correct, there are very many things that are inconsistent between dialects on H5. These have been fixed in Hibernate 6, and I don't think it makes much sense to go back and try to fix them all in H5. People who want better portability between databases should update to H6. |
|
#9323 shows that this works (it has worked since at least H6 and maybe earlier). |
HHH-15228
"RoundFunction" which extends StandardSQLFunction to return BigDecimal